Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fallback solution: Parse "artist-title" with a single dash from file name #3082

Closed
wants to merge 1 commit into from
Closed

Fallback solution: Parse "artist-title" with a single dash from file name #3082

wants to merge 1 commit into from

Conversation

uklotzde
Copy link
Contributor

@uklotzde uklotzde commented Sep 9, 2020

As requested and suggested on Discourse:

https://mixxx.discourse.group/t/metadata-from-filename-not-creating-artist-title/19901

Could be considered a regression from 2.2. This simple fallback solution should not cause any unintended side-effects.

@uklotzde uklotzde added this to the 2.3.0 milestone Sep 9, 2020
@Holzhaus
Copy link
Member

Holzhaus commented Sep 9, 2020

What happens if I have a track named "Some Track (Some Artist Re-Mix).mp3"? I think we should not parse artist-title with a single dash. No magic is far better than broken magic. The person could just tag the files correctly or use perl-rename 's/-/ - /g' *.mp3.

@daschuer
Copy link
Member

daschuer commented Sep 9, 2020

Now we have a complain that "artist-title" is not supported which was the 2.2 state. We had never complains about false positives.
From this point of view let's merge it.

Importing "Some Track (Some Artist Re-Mix).mp3" as Artist: "Some Track (Some Artist Re" Track: "Mix" is probably also not too bad, because you need to adjust the naming anyway.

What do you think?

@Holzhaus
Copy link
Member

Holzhaus commented Sep 9, 2020

Importing "Some Track (Some Artist Re-Mix).mp3" as Artist: "Some Track (Some Artist Re" Track: "Mix" is probably also not too bad, because you need to adjust the naming anyway

Why do you need to adjust the naming anyway? I think that broken parsing is way worse than no parsing at all (even in the artist-title case).

@daschuer
Copy link
Member

daschuer commented Sep 9, 2020

Why do you need to adjust the naming anyway?

Because else I have Title and artist in the title field.

This is finally a personal preference we can't solve in a discussion. I always use spaces, so I don't really mind.
Has this really append to you or is it an artificial example.

The valid argument to me here is that this was reported as a regression and the regression is fixed with this PR.

@Holzhaus
Copy link
Member

Holzhaus commented Sep 9, 2020

Why do you need to adjust the naming anyway?

Because else I have Title and artist in the title field.

No, you only have the title in the title field and no artist at all (as there is no artist is in the filename).

This is finally a personal preference we can't solve in a discussion. I always use spaces, so I don't really mind.

How you name your files is indeed personal preference. Mixxx making wrong assumptions about the file name and applying broken magic where it shouldn't is not personal preference.

Has this really append to you or is it an artificial example.

This is an artificial example because I always tag my files, but AFAIK the default naming scheme for iTunes looks like this: 2-01 Take On Me [Demo Version].mp3, so it's definitely not far-fetched.

I think it's reasonable to assume - (with spaces) separates artist and title, but we don't have enough information to assume that - (without spaces) is the artist/title separator.

The valid argument to me here is that this was reported as a regression and the regression is fixed with this PR.

It's a regression in so far as previous behavior has changed. The original behavior was wrong and I'd consider it a bug. I think it's reasonable to ask the reporter to either fix their file names/tags or live without the magic artist/title parsing.

@daschuer
Copy link
Member

daschuer commented Sep 9, 2020

The original behavior was wrong and I'd consider it a bug

The point is, that we have no evidence for this but for the opposite.

@uklotzde
Copy link
Contributor Author

uklotzde commented Sep 9, 2020

...and those discussions show once again why adding this "feature" in the first place was a bad idea.

I always wanted to delete the code, even if users might complain. Most of them are not even aware that there are better ways for managing metdata. Mixxx prevents them from realizing this.

@uklotzde
Copy link
Contributor Author

???

@daschuer
Copy link
Member

I vote for merge, the work is done and one user will be happy.

@Holzhaus
Copy link
Member

Holzhaus commented Sep 15, 2020

I'm not happy about this. If you really think this is needed instead of users just fixing their tags or naming scheme (which can easily be done with a bulk rename, e. g. using find ~/Music -type f - i name "*.mp3" - exec perl-rename 's/^([^-]+[^ ])-([^ ].*)$/\1 - \2/' {} \;), then we should at least add additional checks to reduce the likelihood of false positives:

  1. The dash must not appear inside parentheses (()) or brackets ([]) to prevent splitting track titles like Foo (Bar Re-Mix).mp3
  2. The dash must not be surrounded with numbers to prevent splitting filenames like 1-2 Some Track.mp3 (default iTunes naming scheme for track 2 on disk 1 of a multi disk album).

Even with these additional rules, I doubt that it is worth it to introduce this complexity (which might still fail to detect undesired splitting) when the user could just fix the naming of the files with minimal effort. But if you deem it necessary I won't vote against the merge then.

@uklotzde
Copy link
Contributor Author

My preferred solution would be to just use the file name excluding the last suffix as the title and get rid of this poor parser.

@uklotzde uklotzde closed this Sep 17, 2020
@uklotzde uklotzde deleted the parse_artist_title branch January 25, 2021 10:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants